Coral-Trino: Migrate UDF operator transformers based on JSON-infra into transformers based on native Java code#355
Closed
yiqiangin wants to merge 1 commit intolinkedin:masterfrom
yiqiangin:json-transformer-migration
Closed
Coral-Trino: Migrate UDF operator transformers based on JSON-infra into transformers based on native Java code#355yiqiangin wants to merge 1 commit intolinkedin:masterfrom yiqiangin:json-transformer-migration
yiqiangin wants to merge 1 commit intolinkedin:masterfrom
yiqiangin:json-transformer-migration
Conversation
…rs based on native Java code
ljfgem
reviewed
Feb 14, 2023
Collaborator
There was a problem hiding this comment.
Thanks @yiqiangin for this PR!
Please add the test part in the PR description.
| * matches the target values in the condition function. | ||
| */ | ||
| public abstract class SourceOperatorMatchSqlCallTransformer extends SqlCallTransformer { | ||
| public static final SqlOperator TIMESTAMP_OPERATOR = |
| } | ||
| }; | ||
|
|
||
| protected final SqlOperator DATE_OPERATOR = new SqlUserDefinedFunction(new SqlIdentifier("date", SqlParserPos.ZERO), |
Collaborator
There was a problem hiding this comment.
Can be static, should be aligned with TIMESTAMP_OPERATOR above.
| // math functions | ||
| new OperatorRenameSqlCallTransformer(SqlStdOperatorTable.RAND, 0, "RANDOM"), | ||
| new JsonTransformSqlCallTransformer(SqlStdOperatorTable.RAND, 1, "RANDOM", "[]", null, null), | ||
| new RandomOperatorWithOneOperandTransformer(), |
Collaborator
There was a problem hiding this comment.
Can we just use OperatorRenameSqlCallTransformer for this case?
Comment on lines
+38
to
+40
| List<SqlNode> newOperands = new ArrayList<>(); | ||
| newOperands.add(sqlCall.getOperandList().get(1)); | ||
| return createCall(TARGET_OPERATOR, newOperands, SqlParserPos.ZERO); |
Collaborator
There was a problem hiding this comment.
Can be simplified to return TARGET_OPERATOR.createCall(sqlCall.getParserPosition(), sqlCall.getOperandList().get(1));, and many similar parts can be simplified in this PR.
|
|
||
| /** | ||
| * This class is a subclass of {@link SourceOperatorMatchSqlCallTransformer} which transforms a Coral SqlCall of "date_add" | ||
| * operator with 2 operands into a Trino SqlCall of an operator named "date_add" |
Collaborator
There was a problem hiding this comment.
Please add examples for each transformer in the class javadoc.
Comment on lines
+36
to
+38
| private static final SqlOperator HIVE_PATTERN_TO_TRINO_OPERATOR = | ||
| new SqlUserDefinedFunction(new SqlIdentifier("hive_pattern_to_trino", SqlParserPos.ZERO), | ||
| FunctionReturnTypes.STRING, null, OperandTypes.STRING, null, null); |
Collaborator
There was a problem hiding this comment.
Can we just use createSqlOperator? Feel that the operand type is not necessary here.
ljfgem
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In order to migrate all the transformers in which any of operatorTransformer, operandTransformer and resultTransformer is defined as a JSON string into the transformers which define the transformation rule in native Java code, the following code changes are made in this PR:
Adding some classes to implement the transformers which extends
SourceOperatorMatchSqlCallTransformerand the specific transformation logic is defined as native Java code includingRemoving the code of
JsonSqlCallTransformerTo be noted that this is the replacement of PR #350